-
Notifications
You must be signed in to change notification settings - Fork 366
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Cherry pick of #5592: Remove redundant log in fillPodInfo/fillServiceInfo#5731: Use labels to enhance records filtering in FA e2e tests#5770: Improve flow-visibility e2e test #5886
Cherry pick of #5592: Remove redundant log in fillPodInfo/fillServiceInfo#5731: Use labels to enhance records filtering in FA e2e tests#5770: Improve flow-visibility e2e test #5886
Conversation
aca72c8
to
28f89a2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
/test-all |
28f89a2
to
b621ce0
Compare
@yuntanghsu why did you have to update the PR? |
I squashed the commits. There were some commits I think they were not squashed well before. |
@tnqn Do we generally squash the commits manually for individual PRs when cherry-picking multiple PRs at once? I'm talking about individual PRs which may have been merged with "Squash and merge" but originally contained multiple commits (see #5592) for an example. |
I always rebase and merge to keep commits consistent, to make it easy to search whether a commit is in a branch. I don't recall I have cherry-picked PRs containing multiple commits (as critical bugfixes don't usually contain multiple commits), but if I were doing it, I would cherry-pick the merge commit manually (d6766cc in this case). |
I think that makes sense. @yuntanghsu you may want to do a manual cherry-pick here, as suggested by Quan. |
…onnection store. (antrea-io#5592) 1. Remove redundant logs in fillPodInfo/fillServiceInfo to avoid Flow Exporter from flooding logs. 2. Add Mark field for deny connections. We were missing this field previously, resulting in trying to fill service information for non-service IPs. 3. Update the DestinationServiceAddress for deny connections when we can find this information in PacketIn. 4. Update e2e/unit tests to verify that the Service-relateds field are filled correctly. Fixes antrea-io#5573 Signed-off-by: Yun-Tang Hsu <[email protected]>
In this commit, we: 1. Add a label to Perftest Pods before initiating traffic in each subtest to filter records in both the IPFIX collector Pod and the ClickHouse. 2. Remove the --since-time flag used during log retrieval from the IPFIX collector Pod in the e2e test. 3. Stop relying on timestamps for record filtering due to potential time discrepancies between the testbed and Kubernetes nodes, which might hinder the retrieval of desired records. Signed-off-by: Yun-Tang Hsu <[email protected]>
1. Changed the order in which we append expired records before exporting them from our exporter. For inter-Node traffic with egress/ingress NP with action drop, we will receive records from both PacketIn and the conntrack table. For the egress case, we need to ensure that the PacketIn record arrives at the FA first, or we will be stuck waiting for correlation. 2. Add checks to verify that records can successfully be sent by the Flow Exporter before sending traffic in e2e tests. 3. Add labels to External subtest to filter useless logs from the IPFIX collector Pod. 4. Confirm the correct addition of a label to a specific Pod after updating the Pod. 5. Remove the octetDeltaCount check and, instead, filter out all records with octetDeltaCount=0 when retrieving records from the IPFIX collector Pod and ClickHouse. 6. Use new version of go-ipfix test collector. The new collector no longer prints all received records. Instead records are saved in-memory, and can be queried over HTTP. The list of records can also be reset with an HTTP call. In this way, we can drastically reduce the time to retrieve records in tests. Signed-off-by: Yun-Tang Hsu <[email protected]>
b621ce0
to
2fe5d84
Compare
@antoninbas I've manually cherry-picked these 3 PRs. Please take a look to see if it is correct. |
It's a bit misleading to reuse the same PR if you didn't use the script. |
What do you mean here? Shouldn't I manually cherry-pick these PRs? Or should I manually do it for the first PR only? |
@yuntanghsu I thought you were going to do it manually for all PRs:
|
I just realize you were saying I should create another PR to replace this one instead of referring to the PRs in this PR. |
replaced by #5900 |
Cherry pick of #5592 #5731 #5770 on release-1.14.
#5592: Remove redundant log in fillPodInfo/fillServiceInfo
#5731: Use labels to enhance records filtering in FA e2e tests
#5770: Improve flow-visibility e2e test
For details on the cherry pick process, see the cherry pick requests page.